Skip to content

Conversation

@lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Jul 14, 2025

Follow-up to #7108

Copilot AI review requested due to automatic review settings July 14, 2025 17:41
@lerouxb lerouxb requested a review from a team as a code owner July 14, 2025 17:41
@lerouxb lerouxb added no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) labels Jul 14, 2025
@github-actions github-actions bot added the fix label Jul 14, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a fallback for retrieving the Chromium version from binaries that don’t support the --versions flag yet.

  • Wraps the JSON parsing of stdout in a try/catch block.
  • Returns MONOREPO_ELECTRON_CHROMIUM_VERSION if parsing fails.
Comments suppressed due to low confidence (2)

packages/compass-e2e-tests/helpers/compass.ts:481

  • Add a unit test to cover the fallback path where JSON.parse fails and MONOREPO_ELECTRON_CHROMIUM_VERSION is returned to ensure this behavior doesn’t regress.
    return MONOREPO_ELECTRON_CHROMIUM_VERSION;

packages/compass-e2e-tests/helpers/compass.ts:476

  • [nitpick] Add a JSDoc comment to explain that this function falls back to MONOREPO_ELECTRON_CHROMIUM_VERSION when parsing the --versions output fails.
async function getChromiumVersionFromBinary(path: string) {

@lerouxb lerouxb force-pushed the fallback-old-binaries branch from 59b48fe to 9a65a9b Compare July 14, 2025 19:30
@lerouxb
Copy link
Contributor Author

lerouxb commented Jul 15, 2025

This does exactly what it should, but I'm pretty sure it is still not working when we cross electron versions. Probably because we start off on one and then end on the other and we don't re-start/re-launch the e2e tests in between.

@lerouxb lerouxb merged commit f6992e0 into main Jul 15, 2025
51 of 58 checks passed
@lerouxb lerouxb deleted the fallback-old-binaries branch July 15, 2025 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants